Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add live feedback storage and statistics #7497

Merged
merged 19 commits into from
Oct 24, 2024
Merged

Add live feedback storage and statistics #7497

merged 19 commits into from
Oct 24, 2024

Conversation

syoopie
Copy link
Contributor

@syoopie syoopie commented Aug 19, 2024 •
edited
Loading

Add support for saving live feedback and displaying it in the assessment statistics page.

Workflow for saving:

  1. GetHelp used, FE sends request to BE.
  2. New entry in course_assessment_live_feedbacks is created and current code states (course_assessment_live_feedback_code) are saved
  3. BE requests for live feedback
    1. If feedback is returned:
      1. Save feedback (course_assessment_live_feedback_comments)
      2. Send feedback to FE
    2. If token is returned:
      1. Send token to FE
      2. FE polls for feedback
      3. FE sends feedback to BE to be saved (course_assessment_live_feedback_comments)

Schema Changes: Added 3 new tables

image

feedback_id under the course_assessments_live_feedbacks table corresponds to the transaction_id provided by codaveri. This is to ensure traceability.

The schema is separated as such so that each feedback request can accomadate multiple different code files, and each code file can have multiple comments.

New Live Feedback statistics page:

Screenshot 2024-09-30 at 2 26 29 PM
  • Table is sorted by descending total
  • Individual questions are colour coded

New Live Feedback History Dialog

image

Other Notes:

This PR also standardises the terminology surrounding the live feedback function.

  • From now on we will only use 2 terms, "Get Help" and "Live Feedback"
  • "Get Help" will be an active, user facing term, which will be used for buttons and their corresponding translations.
  • "Live Feedback" will be the technical term, used for describing the feature to course managers etc., and will also be the term used in the codebase when referring to this feature

@syoopie syoopie changed the title yupei/add live feedback storage and statistics Add live feedback storage and statistics Aug 19, 2024
@syoopie syoopie force-pushed the yupei/add-live-feedback-storage-and-statistics branch 2 times, most recently from 889de1f to 389bdac Compare August 20, 2024 02:47
@bivanalhar bivanalhar requested review from bivanalhar and removed request for bivanalhar August 20, 2024 07:14
@syoopie syoopie force-pushed the yupei/add-live-feedback-storage-and-statistics branch from 389bdac to 063e585 Compare August 20, 2024 07:46
@syoopie syoopie marked this pull request as draft August 27, 2024 10:29
@syoopie syoopie mentioned this pull request Aug 27, 2024
Merged
@syoopie syoopie force-pushed the yupei/add-live-feedback-storage-and-statistics branch 4 times, most recently from 302abf2 to fb7f80f Compare August 29, 2024 05:33
@syoopie syoopie marked this pull request as ready for review August 29, 2024 05:36
bivanalhar
bivanalhar reviewed Sep 3, 2024
View reviewed changes
Copy link
Contributor

@bivanalhar bivanalhar left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @syoopie good job on your PR! It's quite well made..
I have left some comments inside this PR, please attend to them and apply the changes accordingly. If you have any further inquiries, please reply to my comments and if you don't understand how to apply those changes, please don't hesitate to reach out to me

Other than that, here are some advise for your future PRs:

  • please properly name your commit. We have the convention written in our Commit Guideline. Not sure if you have read this before, but please learn about this
  • avoid creating a new commit that are actually merely fixing the commit you just made previously that are still within the same PR. If you have committed several more commits then realising your mistake, instead do git rebase and then edit the respective commit.. Please take a look at the following git rebase guideline to know how to use git rebase properly

Please let me know when you're finished polishing your code, and then request for my re-review for me to look more into your PR

app/models/course/assessment/question.rb Show resolved Hide resolved
db/migrate/20240808083848_add_live_feedback_code_and_comments.rb Show resolved Hide resolved
db/schema.rb Outdated Show resolved Hide resolved
app/views/course/statistics/assessments/main_statistics.json.jbuilder Outdated Show resolved Hide resolved
app/models/course/assessment.rb Show resolved Hide resolved
app/controllers/concerns/course/statistics/live_feedback_concern.rb Show resolved Hide resolved
db/migrate/20240808083848_add_live_feedback_code_and_comments.rb Show resolved Hide resolved
app/models/course/assessment/assessment_ability.rb Show resolved Hide resolved
...les/course/assessment/pages/AssessmentStatistics/LiveFeedbackHistory/LiveFeedbackDetails.tsx Outdated Show resolved Hide resolved
...course/assessment/pages/AssessmentStatistics/LiveFeedbackHistory/LiveFeedbackHistoryPage.tsx Outdated Show resolved Hide resolved
@syoopie syoopie force-pushed the yupei/add-live-feedback-storage-and-statistics branch 5 times, most recently from 0f76c0b to d5e7640 Compare September 3, 2024 09:46
@syoopie syoopie force-pushed the yupei/add-live-feedback-storage-and-statistics branch 7 times, most recently from bd8c12a to 85bdef9 Compare September 3, 2024 14:25
@syoopie syoopie force-pushed the yupei/add-live-feedback-storage-and-statistics branch from 85bdef9 to e1cb9b4 Compare September 4, 2024 03:01
@syoopie syoopie force-pushed the yupei/add-live-feedback-storage-and-statistics branch 2 times, most recently from 43e0ae0 to c85a081 Compare September 17, 2024 06:41
@cysjonathan cysjonathan force-pushed the yupei/add-live-feedback-storage-and-statistics branch from c85a081 to 4035cd4 Compare October 7, 2024 04:04
@cysjonathan cysjonathan force-pushed the yupei/add-live-feedback-storage-and-statistics branch 2 times, most recently from 4414030 to a5f004e Compare October 17, 2024 06:50
cysjonathan
cysjonathan requested changes Oct 18, 2024
View reviewed changes
Copy link
Contributor

@cysjonathan cysjonathan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your last two commits should be squashed into their respective associated commits.

i.e. 2nd last and 3rd last commits are semantically grouped together,
last commit and "feat(live-feedback): add live feedback saving" should be grouped together.

config/routes.rb Show resolved Hide resolved
db/migrate/20240808083848_add_live_feedback_code_and_comments.rb Show resolved Hide resolved
app/controllers/course/statistics/assessments_controller.rb Outdated Show resolved Hide resolved
client/app/bundles/course/assessment/pages/AssessmentStatistics/classNameUtils.ts Show resolved Hide resolved
client/app/api/course/Statistics/AssessmentStatistics.ts Outdated Show resolved Hide resolved
client/app/bundles/course/assessment/pages/AssessmentStatistics/LiveFeedbackStatisticsTable.tsx Outdated Show resolved Hide resolved
client/app/bundles/course/assessment/pages/AssessmentStatistics/LiveFeedbackStatisticsTable.tsx Outdated Show resolved Hide resolved
client/app/bundles/course/assessment/pages/AssessmentStatistics/LiveFeedbackStatisticsTable.tsx Outdated Show resolved Hide resolved
app/controllers/course/assessment/submission/submissions_controller.rb Outdated Show resolved Hide resolved
app/controllers/course/assessment/submission/submissions_controller.rb Show resolved Hide resolved
@syoopie
refactor(main-statistics): add boolean to indicate if live feedback i…
a78ee1b 
…s enabled
@syoopie
feat(live-feedback): add tables for storing live feeedback information
ff5274b 
New Tables:
1. course_assessment_live_feedbacks
2. course_assessment_live_feedback_code
3. course_assessment_live_feedback_comments

Related models have also been updated. These include:
users
course_assessments
course_assessment_questions
@syoopie syoopie force-pushed the yupei/add-live-feedback-storage-and-statistics branch from a5f004e to 3daa72d Compare October 22, 2024 12:41
@syoopie
feat(live-feedback-stats): add backend
2787b4b 
add new route for getting live feedback stats
add new live_feedback_concern
refactored some code in assessments_controller as it was used for live feedback stats as well
add new json return format for live feedback stats
@syoopie
refactor(statistics): classNameUtils
bca2905 
refactored for a wider range of usability
fix erroneous comments
add new function to get class for live feedback cells

previously cells were coloured linearly based on a max number
new colouring only applies this gradient to the lower 75% of the data
upper 25% is all coloured the most intense shade of red
@syoopie
feat(live-feedback-stats): add frontend
3cdd0f4 
add new tab to assessment statistics if live help is enabled for assessment
add new types for live feedback statistics
add call to new route for getting live feedback statistics

refactor functionality for getting jointGroupsName
refactor functionality for translating status

both refactors have their commented out typescript equivalent left inside in file
@syoopie
style(generate-live-feedback): remove unnecessary commented code
08662e3 
this commit used to be to fix a breaking change from codaveri regarding categories,
but the change has been made by another commit that is already merged
the change has been removed from this commit during rebase and only other random changes are left

remove some random commented code in codaveri-async-feedback-service
@syoopie
feat(live-feedback): add live feedback saving
36bdb51 
add frontend logic for returning live feedback once retrieved
add backend logic and routes for saving live feedback
@syoopie
refactor(submission-controller): disable file length restrictions
0ae2a99
@syoopie
refactor(EditorField): add optional cursorStart variable
dc3b32d
@syoopie
refactor(live-feedback-stats): add retrieval of question ids
e0f8db0 
individual live feedback histories need this information to be retrieved
@syoopie
feat(live-feedback-history): add FE and BE
ae6fea0 
add dialog to show live feedback history on the live feedback statistics page
add routes and backend for retrieving live feedback history
@syoopie
test(live-feedback-history): add tests for live feedback history
c4aad24
@syoopie
refactor(assessments-factory)
91e49a6 
Change assessments factory to give unique question weights when creating questions

This is to assist in live feedback tests as weights are needed to order the questions
@syoopie
test(live-feedback-statistics) add tests for live feedback statistics
5424b6c
@syoopie
refactor(assessment-statistics): abstract translations
3d1c0e7 
abstract out the translations of some assessment stsatistics files
add translations and localizations for the associated translations
@syoopie
style(live-feedback): change translations
aeaa54a 
- conform to new standard, all user facing "verbs" are  "Get Help"
- "Get Help" will describe the act of getting live feedback
- all other usage of the terms "Live Help" and "Get Help" will instead be "Live Feedback"
@syoopie
style(live-feedback-history): improve slider styling
40e5d48 
add custom slider with more visible marks and larger height
@syoopie
style(assessment-statistics): use new custom slider
c3bb395
@syoopie
perf(live-feedback-stats): reduce N+1
3e9ba1a 
- sql queries drop from 99 -> ~30
@syoopie syoopie force-pushed the yupei/add-live-feedback-storage-and-statistics branch from 3daa72d to 3e9ba1a Compare October 23, 2024 09:33
@cysjonathan cysjonathan merged commit 6d830d5 into master Oct 24, 2024
12 of 13 checks passed
@cysjonathan cysjonathan deleted the yupei/add-live-feedback-storage-and-statistics branch October 24, 2024 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cysjonathan cysjonathan cysjonathan approved these changes

@bivanalhar bivanalhar bivanalhar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@syoopie @cysjonathan @bivanalhar